Skip to content

[Merged by Bors] - Add bevy_ecs::schedule_v3 module - #6587

Closed
maniwani wants to merge 64 commits into
bevyengine:mainfrom
maniwani:stageless
Closed

[Merged by Bors] - Add bevy_ecs::schedule_v3 module#6587
maniwani wants to merge 64 commits into
bevyengine:mainfrom
maniwani:stageless

Conversation

@maniwani

@maniwani maniwani commented Nov 13, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Objective

Complete the first part of the migration detailed in bevyengine/rfcs#45.

Solution

Add all the new stuff.

TODO

  • Impl tuple methods.
  • Impl chaining.
  • Port ambiguity detection.
  • Write docs.
  • Write more tests.(will do later)
  • Write changelog and examples here?
  • Replace petgraph. (will do later)

@maniwani maniwani added A-ECS Entities, components, systems, and events D-Complex Quite challenging from either a design or technical perspective. Ask for help! C-Feature A new feature, making something new possible labels Nov 13, 2022
@james7132
james7132 self-requested a review November 14, 2022 01:52
@hymm
hymm self-requested a review November 14, 2022 03:02
james7132
james7132 requested changes Nov 14, 2022
View reviewed changes

@james7132 james7132 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is just a cursory scan. A full review to come later.

Comment thread crates/bevy_ecs/src/schedule_v3/executor/mod.rs Outdated
Comment thread crates/bevy_ecs/src/schedule_v3/executor/mod.rs Outdated
Comment thread crates/bevy_ecs/src/schedule_v3/executor/simple.rs Outdated
Comment thread crates/bevy_ecs/src/schedule_v3/executor/simple.rs Outdated
fn init(&mut self, schedule: &SystemSchedule) {
let sys_count = schedule.system_ids.len();
let set_count = schedule.set_ids.len();
self.completed_sets = FixedBitSet::with_capacity(set_count);

@james7132 james7132 Nov 14, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: The sys_count/set_count is not used elsewhere, directly passing them in here is probably as readable as it is now.

Comment thread crates/bevy_ecs/src/schedule_v3/executor/simple.rs Outdated
Comment thread crates/bevy_ecs/src/schedule_v3/executor/single_threaded.rs Outdated
Comment thread crates/bevy_ecs/src/schedule_v3/executor/single_threaded.rs Outdated
Comment thread crates/bevy_ecs/src/schedule_v3/executor/multi_threaded.rs Outdated
Comment thread crates/bevy_ecs/src/schedule_v3/executor/multi_threaded.rs Outdated
Comment thread crates/bevy_ecs/src/schedule_v3/executor/multi_threaded.rs Outdated
@maniwani
maniwani force-pushed the stageless branch 4 times, most recently from 42c96cc to 7754cda Compare November 15, 2022 06:25
@james7132 james7132 added this to the 0.10 milestone Nov 15, 2022
@maniwani
maniwani force-pushed the stageless branch 2 times, most recently from e0bd75b to 1d5f5a4 Compare November 15, 2022 20:43
@maniwani
maniwani requested a review from cart November 15, 2022 20:56
@maniwani
maniwani marked this pull request as ready for review November 15, 2022 20:57
tim-blackbird
tim-blackbird reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/mod.rs Outdated
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/macros/src/lib.rs
derive_label(input, &trait_path, "run_criteria_label")
}

/// Derive macro generating an impl of the trait `ScheduleLabel`.

@alice-i-cecile alice-i-cecile Nov 19, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we able to doc link these traits?

alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/condition.rs Outdated
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/condition.rs Outdated
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/condition.rs
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/config.rs Outdated
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/config.rs Outdated
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/config.rs Outdated
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/config.rs Outdated
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/config.rs Outdated
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/config.rs
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/config.rs Outdated
alice-i-cecile
alice-i-cecile reviewed Nov 19, 2022
View reviewed changes
Comment thread crates/bevy_ecs/src/schedule_v3/executor/single_threaded.rs
@cart

cart commented Jan 16, 2023

Copy link
Copy Markdown
Member

bors r+

bors Bot pushed a commit that referenced this pull request Jan 16, 2023
@maniwani @james7132 @hymm
Add bevy_ecs::schedule_v3 module (#6587)
a5ec9c0 
# Objective

Complete the first part of the migration detailed in bevyengine/rfcs#45.

## Solution

Add all the new stuff.

### TODO

- [x] Impl tuple methods.
- [x] Impl chaining.
- [x] Port ambiguity detection.
- [x] Write docs.
- [x] ~~Write more tests.~~(will do later)
- [ ] Write changelog and examples here?
- [x] ~~Replace `petgraph`.~~ (will do later)



Co-authored-by: james7132 <contact@jamessliu.com>
Co-authored-by: Michael Hsu <mike.hsu@gmail.com>
Co-authored-by: Mike Hsu <mike.hsu@gmail.com>
@mockersf

mockersf commented Jan 17, 2023

Copy link
Copy Markdown
Member

This PR consistently timeout CI: https://github.com/bevyengine/bevy/actions/workflows/ci.yml?query=branch%3Astageless, while other PR are running without issues. This would point to an issue in this one...

@atlv24

atlv24 commented Jan 17, 2023

Copy link
Copy Markdown
Contributor

Running tests using the SingleThreadedExecutor as the default reproduces the hang observed in CI:

test schedule_v3::tests::system_execution::parallel_execution has been running for over 60 seconds

@bors

bors Bot commented Jan 17, 2023

Copy link
Copy Markdown

Timed out.

@mockersf

mockersf commented Jan 17, 2023

Copy link
Copy Markdown
Member

bors r+

@bors

ghost commented Jan 17, 2023

Copy link
Copy Markdown

Pull request successfully merged into main.

Build succeeded:

@bors bors Bot changed the title Add bevy_ecs::schedule_v3 module [Merged by Bors] - Add bevy_ecs::schedule_v3 module Jan 17, 2023
@bors bors Bot closed this Jan 17, 2023
@alice-i-cecile alice-i-cecile mentioned this pull request Jan 18, 2023
Closed
33 tasks
@james7132 james7132 mentioned this pull request Jan 22, 2023
Closed
@alice-i-cecile alice-i-cecile mentioned this pull request Jan 31, 2023
Closed
This was referenced Feb 6, 2023
Closed
Closed
@maniwani
maniwani deleted the stageless branch February 11, 2023 04:03
@cart cart mentioned this pull request Feb 15, 2023
Merged
@cart cart mentioned this pull request Feb 24, 2023
Closed
@farnoy farnoy mentioned this pull request Mar 10, 2023
Closed
@nicopap nicopap mentioned this pull request Apr 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cart cart cart approved these changes
@james7132 james7132 james7132 approved these changes
@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes
+8 more reviewers
@hymm hymm hymm left review comments
@Guvante Guvante Guvante left review comments
@Carter0 Carter0 Carter0 left review comments
@BoxyUwU BoxyUwU BoxyUwU left review comments
@Shatur Shatur Shatur left review comments
@tim-blackbird tim-blackbird tim-blackbird left review comments
@DasLixou DasLixou DasLixou left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

@cart's attention
Archived in project
Scheduling
Archived in project

Milestone

0.10

Development

Successfully merging this pull request may close these issues.