Skip to content

Expose system accesses and filters in BRP schedule.graph#24743

Open
Zeophlite wants to merge 16 commits into
bevyengine:mainfrom
Zeophlite:schedule-data-accesses
Open

Expose system accesses and filters in BRP schedule.graph#24743
Zeophlite wants to merge 16 commits into
bevyengine:mainfrom
Zeophlite:schedule-data-accesses

Conversation

@Zeophlite

@Zeophlite Zeophlite commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Objective

  • ScheduleData in bevy_dev_tools, and correspondingly schedule.graph in BRP, give only components related to system conflicts, not to accesses and filters, or required components - this is needed for schedule visualisation

Solution

  • Add accesses, filters, and required components to ScheduleData
  • Refactor tracking of components

Testing

  • cargo run --example server --features="bevy_remote" in terminal 1
  • curl -d'{"jsonrpc":"2.0","method":"schedule.graph","id":1,"params":{"schedule_label":"Update"}}' -X POST -H "Accept: applcation/json" -H "Content-Type: application/json" http://127.0.0.1:15702 > update3.json in terminal 2 (see migration guide)

@Zeophlite Zeophlite added A-ECS Entities, components, systems, and events A-Dev-Tools Tools used to debug Bevy applications. D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 24, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in ECS Jun 24, 2026
@Zeophlite Zeophlite force-pushed the schedule-data-accesses branch 3 times, most recently from dd9ee4d to 5c8f7ba Compare June 24, 2026 15:39
@Zeophlite Zeophlite force-pushed the schedule-data-accesses branch from 5c8f7ba to 93df812 Compare June 24, 2026 17:11
@Zeophlite Zeophlite added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 24, 2026
@Zeophlite Zeophlite marked this pull request as ready for review June 24, 2026 17:13
Comment thread crates/bevy_ecs/src/query/access.rs Outdated
@andriyDev andriyDev self-requested a review June 24, 2026 18:43
@Zeophlite Zeophlite force-pushed the schedule-data-accesses branch from b2514a1 to 9c380e2 Compare June 25, 2026 15:45
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_ecs/src/schedule/node.rs Outdated
Comment thread crates/bevy_ecs/src/schedule/schedule.rs Outdated
@Zeophlite

Copy link
Copy Markdown
Contributor Author

Note: docs copied from underlying objects, may reduce them later

@github-actions

Copy link
Copy Markdown
Contributor

You added a new feature but didn't add a description for it. Please update the root Cargo.toml file.

@Zeophlite

Zeophlite commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

I've removed invertible support for now, will re-add as follow-up PR when #24771 or #24809 is merged

@alice-i-cecile alice-i-cecile added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes.

Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated

@alice-i-cecile alice-i-cecile 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.

LGTM once there's a migration guide :)

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 6, 2026
@Zeophlite Zeophlite added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jul 6, 2026
@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Jul 6, 2026
@alice-i-cecile

Copy link
Copy Markdown
Member

Needs a second approval still :)

@chescock chescock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems like valuable data to expose! I'm still skeptical about exposing reads_inverted and writes_inverted in this way, but I'm willing to approve anyway if others argue that it's the right approach.

pub struct AccessData {
/// All accessed components, or forbidden components if
/// `Self::read_and_writes_inverted` is set.
pub read_and_writes: Vec<u32>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'll note again that exposing a list that is either one thing or its opposite is very confusing. I'm not familiar with the BRP conventions for serializing enums, though. Would it work serialize this as separate inclusion and exclusion lists? Something like

    /// If `false`, this reads all components in `reads_included`.
    /// If `true`, this reads all components *except* those in `reads_excluded`.
    pub reads_unbounded: bool,
    /// These components are *always* read,
    /// but if `reads_unbounded` then this is empty and not exhaustive.
    pub reads_included: Vec<u32>,
    /// These components are *never* read,
    /// but if `!reads_unbounded` then this is empty and not exhaustive.
    pub reads_excluded: Vec<u32>,

would be a little harder to misinterpret.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Once #24771 or #24809 lands, I will update to match

Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs
Comment thread crates/bevy_dev_tools/src/schedule_data/serde.rs Outdated
@Zeophlite Zeophlite force-pushed the schedule-data-accesses branch from 64eaa4e to 2c13ca6 Compare July 7, 2026 13:43
@Zeophlite Zeophlite requested a review from chescock July 7, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Dev-Tools Tools used to debug Bevy applications. A-ECS Entities, components, systems, and events D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

4 participants