bevyengine / bevy

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

Misleading docs on duplicate required component constructors #16494

Open benfrankel opened 17 hours ago

benfrankel commented 17 hours ago

The required components documentation describes the algorithm for resolving multiple constructors for the same component in a required components tree as follows:

In general, this shouldn’t happen often, but when it does the algorithm is simple and predictable:

  1. Use all of the constructors (including default constructors) directly defined in the spawned component’s require list
  2. In the order the requires are defined in #[require()], recursively visit the require list of each of the components in the list (this is a Depth First Search). When a constructor is found, it will only be used if one has not already been found.

However, this is not what actually happens. For example, consider this case:

#[derive(Component)]
#[require(B)]
struct A;

#[derive(Component, Default)]
#[require(C, D(|| D(1)))]
struct B;

#[derive(Component, Default)]
#[require(D(|| D(2)))]
struct C;

#[derive(Component)]
struct D(u8);

Now suppose we spawn A. Following the steps described in the docs to determine which constructor will be used for D:

  1. The "spawned component" is A, which does not have a constructor for D, so go to step 2.
  2. A DFS will visit the components in the order A B C D(2) D(1). So when D(2) is visited, it will be used because D hasn't been found yet; and when D(1) is visited, it will not be used because D has been found already.

This suggests that D(2) will be used as the constructor. But that's incorrect: D(1) is actually used.

alice-i-cecile commented 14 hours ago

@cart, do we prefer the implemented behavior or the documented behavior? I think the implemented behavior is better: more locally defined is more likely to be relevant.