Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link to Main schedule docs from other schedules #10691

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

stepancheg
Copy link
Contributor

I incorrectly assumed that moving a system from Update to FixedUpdate would simplify logic without hurting performance.

But this is not the case: if there's single-threaded long computation in the FixedUpdate, the machine won't do anything else in parallel with it. Which might be not what users expect.

So this PR adds a note. But maybe it is obvious, I don't know.

@alice-i-cecile
Copy link
Member

Hmm, I don't think this is a helpful place for this. IMO the better solution is good module or book-level docs on the basics of scheduling.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events X-Controversial There is active debate or serious implications around merging this PR labels Nov 22, 2023
@stepancheg
Copy link
Contributor Author

I agree, this is not ideal.

Maybe I could replaced it with something like "see the Main schedule for more details how schedules are run", which explains what's going on quite clearly?

@@ -74,6 +74,10 @@ pub struct RunFixedUpdateLoop;
///
/// Frequency of execution is configured by inserting `Time<Fixed>` resource, 64 Hz by default.
/// See [this example](https://github.com/bevyengine/bevy/blob/latest/examples/time/time.rs).
///
/// Note that schedules are not run in parallel, in particular,
/// systems in the `FixedUpdate` schedule are not run in parallel with systems
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"systems in the FixedUpdate schedule are all run before systems in the Update schedule"

next question people are going to ask is: "okay, what's the order of execution then?", and you can change just a single word in docs to immediately answer it

@stepancheg
Copy link
Contributor Author

Changed wording/links to main.

@alice-i-cecile alice-i-cecile removed the X-Controversial There is active debate or serious implications around merging this PR label Nov 22, 2023
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 22, 2023
@alice-i-cecile alice-i-cecile changed the title Explain that FixedUpdate is not parallel with Update Link to Main schedule docs from other schedules Nov 22, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Nov 22, 2023
Merged via the queue into bevyengine:main with commit 85b6326 Nov 22, 2023
23 checks passed
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
I incorrectly assumed that moving a system from `Update` to
`FixedUpdate` would simplify logic without hurting performance.

But this is not the case: if there's single-threaded long computation in
the `FixedUpdate`, the machine won't do anything else in parallel with
it. Which might be not what users expect.

So this PR adds a note. But maybe it is obvious, I don't know.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants