bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Deprecate dynamic plugins

Open BD103 opened this issue 4 months ago • 8 comments

Objective

  • The current implementation for dynamic plugins is unsound. Please see #11969 for background and justification.
  • Closes #11969 and closes #13073.

Solution

  • Deprecate all dynamic plugin items for Bevy 0.14, with plans to remove them for Bevy 0.15.

Discussion

One thing I want to make clear is that I'm not opposed to dynamic plugins in general. I think they can be handy, especially for DLC and modding, but I think the current system is the wrong approach. It's too much of a footgun for the meager benefit is provides.


Changelog

  • Deprecated the current dynamic plugin system.
    • Dynamic plugins will be removed in Bevy 0.15. For now you can continue using them by marking your code with #[allow(deprecated)].

Migration Guide

If possible, remove all usage of dynamic plugins.

// Old
#[derive(DynamicPlugin)]
pub struct MyPlugin;

App::new()
    .load_plugin("path/to/plugin")
    .run();

// New
pub struct MyPlugin;

App::new()
    .add_plugins(MyPlugin)
    .run();

If you are unable to do that, you may temporarily silence the deprecation warnings.

#[allow(deprecated)]

Please note that the current dynamic plugin system will be removed by the next major Bevy release, so you will have to migrate eventually. You may be interested in these safer alternatives:

BD103 avatar Apr 23 '24 22:04 BD103

I've added the "needs release notes" label, though I can be convinced otherwise. I don't know how large of an impact this change will make.

BD103 avatar Apr 23 '24 22:04 BD103

Previously attempted in https://github.com/bevyengine/bevy/pull/3893

See https://github.com/bevyengine/bevy/pull/3893#issuecomment-1055247308 for Cart's opinion at the time.

My position remains about the same as it was then:

  1. Improving this crate is a poor use of our organizational time currently
  2. There have been approximately 0 meaningful PRs to the crate ever
  3. It presents a misleading and unsafe rabbit hole for new users

Given the presence of testing showing indications of UB during reasonable use, I think now's as good a time as any to officially drop support.

alice-i-cecile avatar Apr 23 '24 22:04 alice-i-cecile

maybe the deprecated comment could redirect to something like "go comment on this pr if you see this message" to encourage potential users to report back

mockersf avatar Apr 24 '24 04:04 mockersf

I'd also like to express my support for this change. Aside from the correctness issues, the current version also effectively blocks some advanced plugin build ordering/order-independence features.

NthTensor avatar Apr 24 '24 11:04 NthTensor

maybe the deprecated comment could redirect to something like "go comment on this pr if you see this message" to encourage potential users to report back

I added a deprecation notice in 1b97d99575e322998ec4582cfc16573f4abb4b2e. Hopefully if the alternatives listed do not support someone's use-case, they will comment here. (It may help designing a new API!)

BD103 avatar Apr 24 '24 12:04 BD103

I am sorry, how again is this controversial? I haven't seen any comment indicating such.

nerdachse avatar May 03 '24 09:05 nerdachse

This was controversial the last time it was tried, which makes this sort of controversial-by-default.

NthTensor avatar May 03 '24 09:05 NthTensor

I am sorry, how again is this controversial? I haven't seen any comment indicating such.

@cart disagreed with this change last time it was attempted. (https://github.com/bevyengine/bevy/pull/3893#issuecomment-1055247308) No one has picked up dynamic plugins and tried to improve it since deprecating it was last attempted.

Furthermore, I think bevy_dynamic_plugin even existing in the main Bevy project is encouraging users to use it instead of making their own, better alternatives. As someone mentioned on Discord, Bevy has a reputation for reliability (partially due to being written in Rust). I would argue that dynamic plugins are quite unreliable, hurting this reputation. They are simple enough that if someone wanted to use them, they could just copy the code. I think they should do this, because it puts the responsibility on them and not us.

BD103 avatar May 03 '24 12:05 BD103