bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
35.04k stars 3.44k forks source link

Automatically detect and warn about duplicate Bevy (or Bevy-depending) crates #11304

Open alice-i-cecile opened 8 months ago

alice-i-cecile commented 8 months ago

What problem does this solve or what need does it fill?

When working with third party dependencies, it's very common to get inscrutable errors about how the trait Plugin (or Component or Resource) isn't implemented.

This gets particularly nasty when you're depending on two different versions of a Bevy plugin that use their own component or resource types to communicate (raised by @aevyrie). If they're semver-incompatible, but rely on the same version of Bevy, everything will compile correctly, but nothing will work.

The two types will be different, but with the same name: you'll get duplicate components and all interop will silently break.

What solution would you like?

Borrowing from cargo-deny, we should inspect the Cargo.lock (or perhaps Cargo.toml) of the user's crate, and warn if problems are detected.

We want to warn if:

  1. Multiple versions of bevy are in tree.
  2. Multiple versions of the same crate which depends on bevy, bevy_app or bevy_ecs are in tree.

This should be done during App initialization, before we run it for the first time, and panic if a problem is detected. This needs to be done as part of the App lifecycle to ensure it's always caught, and should panic, as this is a critical error that should never be ignored.

What alternative(s) have you considered?

The first problem can be interactively detected with

cargo tree -i bevy

Similarly, you can scan for duplicates in the same way.

At a CI level, one can use cargo-deny, manually adding all of your Bevy plugin crates to the "deny duplicate dependencies list". However, this will only catch the problem once you hit CI: making a PR or checking in mysteriously broken code is an unlikely pattern.

Ultimately though, neither of these solutions solve it for the users who need it most: users who don't know they have a problem, either due to subtle effects or being new to Bevy and Rust.

We could instead have some editor-based process to check for this, but a) that doesn't currently exist and b) not all users of Bevy will use the editor, and this check is just as important to them.

aevyrie commented 7 months ago

This gets particularly nasty when you're depending on two different versions of a Bevy plugin that use their own component or resource types to communicate (raised by @aevyrie). If they're semver-incompatible, but rely on the same version of Bevy, everything will compile correctly, but nothing will work.

To add to this, we already have tools that catch this class of error for Events and Resources, this only affects Components because they are registered implicitly.

An application usually runs into this type of issue if it is using two crates (often internal) that point to two different versions of a library crate. If they are using events or resources, this will result in an error at startup. This is because you can only add a plugin in one place, so if the crate adding the plugin is using a different version from the crates using the plugin, you will get a panic at startup saying that the event or resource have not been added.

However, this does not happen for components, because plugins do not register them, they are implicitly added. This results in issues where things just... don't work, but silently.

I know of two very common cases of this:

  1. The first is when doing a dependency update, and pulling in two version of a component, but only one version has a plugin that is actually doing anything with the component. If you add mylib v0.1 and its plugin in one crate, and only use mylib v0.2's components a second crate, the components in the second crate will be silently idempotent, which can be very difficult to debug.
  2. The other case is common with learners. They will, for example, pull in bevy_mod_picking and add all the required components, but fail to add the plugin. This results in the app running but mysteriously not working.

For both of these cases, if we required components to be registered in the same way we do with events and resources, we could give helpful messages and panic until it is fixed.

BD103 commented 6 months ago

Perhaps a static could be defined with an exported name. If Bevy gets linked twice, then it will raise an error:

#[no_mangle]
#[export_name = "bevy"]
pub static BEVY: bool = true;

// Uncommenting this would raise an error, due to the same export_name.
// #[no_mangle]
// #[export_name = "bevy"]
// pub static BEVY2: bool = true;
error: symbol `bevy` is already defined
 --> src/lib1.rs:3:1
  |
3 | pub static BEVY: bool = true;
  | ^^^^^^^^^^^^^^^^^^^^^^

error: could not compile `bevy-experiments` (bin "bevy-experiments") due to 1 previous error

Maybe BEVY could be a string and export the version number, so the error makes sense. I don't know which crate this would be put in, though.