Skip to content

Conversation

@maniwani
Copy link
Contributor

@maniwani maniwani commented Nov 13, 2022

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
Copy link
Member

@james7132 james7132 left a comment

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.

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);
Copy link
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.

@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
derive_label(input, &trait_path, "run_criteria_label")
}

/// Derive macro generating an impl of the trait `ScheduleLabel`.
Copy link
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?

Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

My previous comments are all resolved and I think this is in a good spot now! One more small optimization that I think we can do now (see below), but I think we're good to go!

@cart
Copy link
Member

cart commented Jan 16, 2023

bors r+

@mockersf
Copy link
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
Copy link
Contributor

atlv24 commented Jan 17, 2023

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
Copy link
Contributor

bors bot commented Jan 17, 2023

Timed out.

@mockersf
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 17, 2023

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-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

Archived in project
Archived in project

Development

Successfully merging this pull request may close these issues.