Do not add TimePlugin

[elsid]

If application adds TimePlugin too, this breaks time delta computation. See this comment for more details.

Applications should add all required plugins before adding heron plugins.

[jcornaz]

Hi,

Yes, indeed you're right. Thank you very much for the report and for your contribution!

If possible, I would prefer to improve the situation without making a breaking change. I think it could be achieved by creating new plugins and deprecating the old ones. In concept that could look like this:

#[derive(Debug, Default)]
#[non_exhaustive]
pub struct RapierPluginSlim;
impl Plugin for RapierPluginSlim {
  fn build(&mut self, app: &mut App) { /* move here the installation, except for the TimePlugin */ }
}

#[deprecated(reason = "Please use `RapierPluginSlim`instead")]
pub struct RapierPlugin;
impl Plugin for RapierPlugin {
  fn build(&mut self, app: &mut App) {
    app.add_plugin(TimePlugin::default()).add_plugin(RapierPluginStruct::default());
  }
}

The name RapierPluginSlim is maybe not great, if you got a better idea you can go for it. But there is no need to bikeshed too much as I would even consider RapierPluginV2 to be better than a breaking change, and it may easily be improved later without breaking the API.

[elsid]

I don't agree to make the change without breaking the API. Let me explain.

I've found the issue when tried to update my project from bevy 0.7 and heron 3.1.0 to 0.8.1 and 4.0.2 respectively. For me 4.0.2 did a breaking change because of adding TimePlugin by RapierPlugin (TimePlugin didn't exist before bevy 0.8). I'm not sure whether it was it intentional but I think I'm not the only one who faced this problem. Bevy documentation suggests to use its plugin groups like MinimalPlugins and DefaultPlugins depending on the use case and both add TimePlugin. I expect most of applications to follow this approach. Maybe there are applications which cherry-pick specific plugins and don't use plugin groups. IIUC your concern is about these applications which will break by this pull request because they don't add TimePlugin and rely on RapierPlugin doing it. I'm not sure it's a good idea to avoid breaking API change just to keep current potentially incorrect behaviour for the benefit of such projects. This only complicates the update from 3.1.0 to 4 for regular users.

Fundamental problem here is probably a lack of mechanism to prevent plugin duplication in bevy or basically there is no plugin dependency management (bevyengine/bevy#69). I think #301 did a mistake without understanding the problem in attempt to fix tests. Unfortunatelly there is no cheap fix for such mistakes. Breaking API change could save time for developers who are not aware of the problem yet which I think is better than providing smooth update for applications already using 4.0.2.

[jcornaz]

I actually changed my mind, and I accept this change as-is despite being a breaking change. After the release of heron 5 I'll publish a heron 4.1 leveraging the semver trick to make heron 4 compatible with heron 5.

Sorry for my epidermic reaction.

Bevy's lack of care for API stability (which lead me to discontinue heron) combined to my experience of having to deal with breaking changes in languages that are much less good than rust, I started to have a very strong (maybe extreme position) against breaking changes.

I still mostly think that breaking changes are evil, and that they are overused by the software industry. So many of them could be avoided very easily.

However, I now realize that for a rust library (but only for a rust library), a breaking change maybe acceptable if (and only if):

• It makes the API significantly better than any non-breaking change I can think of. (your change does fulfill this requirement IMO)
• The semver trick can be used (and is used) to keep the previous major version mostly compatible with the new one (enabling a smoother upgrade path for consumers, as they may use both major version together for a while)
• The breaking change is published immediatly (it shouldn't wait for 3 months to be batched with 100's of other breaking changes...)

[github-actions]

🎉 This issue has been resolved in version 5.0.0 🎉