[Merged by Bors] - Unique plugin

[mockersf]

Objective

• Make it impossible to add a plugin twice
• This is going to be more a risk for plugins with configurations, to avoid things like App::new().add_plugins(DefaultPlugins).add_plugin(ImagePlugin::default_nearest())

Solution

• Panic when a plugin is added twice
• It's still possible to mark a plugin as not unique by overriding is_unique
• Simpler version of can only add a plugin once #3988 (not simpler anymore because of how PluginGroupBuilder implements PluginGroup)

[alice-i-cecile]

Nice and simple, very useful. Definitely agree with the decision to make plugins unique by default.

I'd like:

1. Docs about the behavior of generic plugins.
2. A should_panic test for duplicate plugins.
3. A test that allowing duplicates works.
4. A test that adding generic plugins with different type parameters works.

Looks like #3988 has tests, so steal those :p

[alice-i-cecile]

Left some docs for you :) I won't block on them, but do think they should be added.

[james7132]

Looks great. Some minor changes regarding startup performance and usability.

[alice-i-cecile]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-and-install-on-iOS
• build-android
• build (macos-latest)
• build (ubuntu-latest)
• build-wasm
• build (windows-latest)
• build-without-default-features (bevy)
• build-without-default-features (bevy_ecs)
• build-without-default-features (bevy_reflect)
• check-compiles
• check-doc
• check-missing-examples-in-docs
• ci
• markdownlint
• run-examples
• run-examples-on-wasm
• run-examples-on-windows-dx12

[LarsDu]

This seems to have broken my latest builds:
https://github.com/LarsDu/StarRust/tree/bevy-0.9-unstable

Update:

• The build broke due to two things:
  • I actually did load the WeaponPlugin twice by accident
  • One of my Plugins was named ScenePlugin which is identical to one of the plugins loaded in DefaultPlugins

Perhaps there should be a followup PR to make it so that you can't name two different plugins the same thing?

[ramirezmike]

One of my Plugins was named ScenePlugin which is identical to one of the plugins loaded in DefaultPlugins

I saw you mention this in the discord and thought that was odd.. Like, if the code is using the type name to identify uniqueness, I would expect it would be the fully qualified name so that you wouldn't get collisions across crates.

It looks like this PR uses the Plugin trait's name function which uses std::any::type_name.. which does seem to output the fully qualified name, so @LarsDu I'm curious if it did actually panic because of ScenePlugin..

Additionally, @mockersf (and maybe @alice-i-cecile?):

1. the Plugin trait's doc comment says /// Configures a name for the [Plugin] which is _primarily_ used for debugging. which is somewhat less true now, right?
2. std::any::type_name has several warnings about the uniqueness guarantees of its output and also makes no guarantee that it will stay the same across versions

again, just saw this and thought 🤔 and wanted to make sure this was all OK

[alice-i-cecile]

Yeah, I think that we should update those docs to reflect its use in this feature.

I'm fine using std::any::type_name for this though, especially since there's multiple escape hatches.

[tim-blackbird]

I don't like the thought of Bevy or third party crates adding a new Plugin(even a private one!) breaking user code because the name overlaps.
Even worse, imagine 2 different third party crates adding a plugin with the same name, now you can't use them together without one of them changing their code and publishing a new version :(

How about identifying unique plugins by TypeId, or perhaps a combination of TypeId and the name?

[ramirezmike]

@devil-ira

Based on the documentation of std::any::type_name, I don't think this happens. It looks like it outputs the fully qualified name (for now :eyes:). Your concern is exactly why I brought it up because @LarsDu said this happened when they had a plugin named ScenePlugin in their user code. I think we'll want to wait until they confirm that's actually what happened.

[mockersf]

Bevy's own ScenePlugin is registered as bevy_scene::ScenePlugin