[Merged by Bors] - expose stages and system containers #1647
Conversation
alice-i-cecile
added
A-ECS
| @@ -9,7 +9,7 @@ use crate::{ | |||
| }; | |||
| use std::{borrow::Cow, ptr::NonNull}; | |||
|
|
|||
| pub(super) trait SystemContainer { | |||
| pub trait SystemContainer { | |||
There was a problem hiding this comment.
This wasn't intended to be exposed, but I suppose it won't be a problem, as long as there's no mutable access. Ideally, it should be documented as well.
There was a problem hiding this comment.
The only thing that's mutable is set_dependencies, but the dependencies are only used for ambiguity checking so exposing that at least isn't unsound.
There was a problem hiding this comment.
We could add #[doc(hidden)] to some methods as well if we don't want to expose them.
There was a problem hiding this comment.
.dependencies() is used for ordering, too. It would indeed be best annotate it and .set_dependencies() with #[doc(hidden)]: it's an implementation detail that just doesn't make sense to external users.
Co-authored-by: Alexander Sepity <ratysz@gmail.com>
This reverts commit a1a4d8d.
|
Another nit: the new |
`set_dependencies`
|
What does "not fully-formed" mean in this context? |
|
Systems are not initialized or sorted until the first run. There will most likely be more things in the future, like resolving and validating various labels (e.g. run criteria labels I'm working on). |
| @@ -167,6 +167,26 @@ impl SystemStage { | |||
| self.add_system_to_set(system, 0) | |||
| } | |||
|
|
|||
| /// Topologically sorted parallel systems. | |||
| /// | |||
| /// Note that this method won't output fully-formed data until the stage has been ran at least once. | |||
There was a problem hiding this comment.
This applies to all systems.
| @@ -167,6 +167,32 @@ impl SystemStage { | |||
| self.add_system_to_set(system, 0) | |||
| } | |||
|
|
|||
| /// Topologically sorted parallel systems. | |||
| /// | |||
| /// Note that systems won't be fully-formed until the stage has been ran at least once. | |||
There was a problem hiding this comment.
Nit: "run" is an "irregular verb" that behaves weirdly in cases like "has been". Every case of "ran" in these docs should be replaced with "run". English is a bad language :)
There was a problem hiding this comment.
I'm a native speaker and I still had to look this up to be sure my intuition wasn't wrong.
There was a problem hiding this comment.
Hmm looks like @Ratysz and I have differing opinions here (just saw the older threads here). Heres a source: https://ell.stackexchange.com/questions/133221/when-to-use-run-vs-when-to-use-ran
There was a problem hiding this comment.
Ah wait @Ratysz's comments are for a different case: "that want to be ran". I don't know how to handle that one. My head hurts :)
There was a problem hiding this comment.
There was a problem hiding this comment.
I think all cases should be "run" here.
There was a problem hiding this comment.
... wow. Clearly, we have to rewrite English in Rust. Apologies for adding to the confusion, @jakobhellermann!
I don't have links, I'm just going with my bilingual intuition - and it's throwing up its hands into the air right now.
|
bors r+ |
This allows third-party plugins to analyze the schedule, e.g. `bevy_mod_picking` can [display a schedule graph](https://github.com/jakobhellermann/bevy_mod_debugdump/tree/schedule-graph#schedule-graph): 
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
This allows third-party plugins to analyze the schedule, e.g.
bevy_mod_pickingcan display a schedule graph: