bevy
bevy copied to clipboard
Add missing schedules in `add_systems` doc (#11814)
Objective
Fixes (#11814)
Solution
I added Update where it was missing in the rustdoc.
Uncovered problem
The previous doc was mocking the app as @hymm said:
app doesn't actually exist in bevy_ecs. It's just mocked in the doc comments as a schedule.
Proposed solutions
-
@pablo-lua suggested to just rename
appinscheduleand let the example be correct without the changes of the PR as it is, in fact, aschedule -
@hymm suggested to move the
add_systemsimpl onAppontoWorldand just use that on the impl onApp
Welcome, new contributor!
Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨
I'm not sure I understand why, when I try to run the rustdoc, I get:
error[E0425]: cannot find value `Update` in this scope
--> crates/bevy_ecs/src/system/combinator.rs:57:5
|
40 | Update,
| ^^^^^^ not found in this scope
error[E0061]: this method takes 1 argument but 2 arguments were supplied
--> crates/bevy_ecs/src/system/combinator.rs:56:5
|
39 | app.add_systems(
| ^^^^^^^^^^^
40 | Update,
| ____________-
41 | |/ my_system.run_if(Xor::new(
42 | || IntoSystem::into_system(resource_equals(A(1))),
43 | || IntoSystem::into_system(resource_equals(B(1))),
44 | || // The name of the combined system.
45 | || std::borrow::Cow::Borrowed("a ^ b"),
46 | || )));
| || -
| ||__|
| |__help: remove the extra argument
| unexpected argument of type `NodeConfigs<Box<(dyn bevy_ecs::system::System<In = (), Out = ()> + 'static)>>`
|
note: method defined here
--> /home/purfakt/dev/perso/bevy/crates/bevy_ecs/src/schedule/schedule.rs:249:12
|
249 | pub fn add_systems<M>(&mut self, systems: impl IntoSystemConfigs<M>) -> &mut Self {
| ^^^^^^^^^^^
error: aborting due to 2 previous errors
This is because app doesn't actually exist in bevy_ecs. It's just mocked in the doc comments as a schedule. Not sure how to fix this.
I think this can't be solved unless we import bevy_app into bevy_ecs (that would probably defeat the purpose of bevy_ecs being independent) An possible option is changing the var name from app to something else
Here we have the definition of the var:
/// # let mut app = Schedule::default();
We would first remove the # to clear that we are not talking about the App here and rename app to schedule
/// let mut schedule = Schedule::default();
That would clear up the problem, I think
We could move the add_systems impl on App onto world and just use that on the impl on App. More code duplication, but might be worth it.
Oh I see. I'm not sure if I'm capable or entitled to make this change.
I didn't expect my first PR to be tagged controversial. Especially, when it was just a documentation update!
Oh I see. I'm not sure if I'm capable or entitled to make this change.
I didn't expect my first PR to be tagged controversial. Especially, when it was just a documentation update!
Well, this means that the issue is rather good, as this shows that there is a misunderstood going about what is the app that the documentation is talking about, a third opinion might be good here in regarding of what to do for sure.
I wouldn't mind having this pr change the examples in bevy_ecs to use schedule, so at least they're correct. We can make an issue for a follow up to discus adding a add_systems method to world.